Skip to content

SONARJAVA-6703 Implement new rule S9133 - #5862

Draft
romainbrenguier wants to merge 3 commits into
masterfrom
new-rule/SONARJAVA-6703-S9133
Draft

SONARJAVA-6703 Implement new rule S9133#5862
romainbrenguier wants to merge 3 commits into
masterfrom
new-rule/SONARJAVA-6703-S9133

Conversation

@romainbrenguier

@romainbrenguier romainbrenguier commented Aug 3, 2026

Copy link
Copy Markdown
Contributor

Detect hard-coded floating-point literals that approximate well-known mathematical constants (pi, e, sqrt(2), ln(2)) and suggest using the corresponding Math class constants or expressions instead.

Part of

@romainbrenguier
Implement new rule S9133
b2a4577 
Detect hard-coded floating-point literals that approximate well-known
mathematical constants (pi, e, sqrt(2), ln(2)) and suggest using the
corresponding Math class constants or expressions instead.
@hashicorp-vault-sonar-prod

hashicorp-vault-sonar-prod Bot commented Aug 3, 2026 •
edited by atlassian Bot
Loading

Copy link
Copy Markdown
Contributor

SONARJAVA-6703

gitar-bot[bot]
gitar-bot Bot reviewed Aug 3, 2026
View reviewed changes
Comment thread java-checks/src/main/java/org/sonar/java/checks/HardcodedMathConstantCheck.java Outdated
gitar-bot[bot]
gitar-bot Bot reviewed Aug 3, 2026
View reviewed changes
Comment thread java-checks/src/main/java/org/sonar/java/checks/HardcodedMathConstantCheck.java
@github-actions

github-actions Bot commented Aug 3, 2026

Copy link
Copy Markdown
Contributor

❌ Ruling needs updating. A fix PR has been created: #5863

Please review and merge it into your branch.

@github-actions github-actions Bot mentioned this pull request Aug 3, 2026
Merged
romainbrenguier pushed a commit that referenced this pull request Aug 3, 2026
@github-actions
Update ruling results for PR #5862 (#5863)
5743ab9 
Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
@github-actions

github-actions Bot commented Aug 3, 2026

Copy link
Copy Markdown
Contributor

Ruling Diff Summary

Detected changes in 1 rule files: 0 issues removed, 18 issues added.

S9133 (java) on sonar-server - 0 issues removed, 18 issues added - new ruling file

Added src/test/java/org/sonar/server/computation/task/projectanalysis/issue/TrackerRawInputFactoryTest.java (line 103)

(source file not found at this revision: src/test/java/org/sonar/server/computation/task/projectanalysis/issue/TrackerRawInputFactoryTest.java)

Added src/test/java/org/sonar/server/computation/task/projectanalysis/issue/TrackerRawInputFactoryTest.java (line 116)

(source file not found at this revision: src/test/java/org/sonar/server/computation/task/projectanalysis/issue/TrackerRawInputFactoryTest.java)

Added src/test/java/org/sonar/server/computation/task/projectanalysis/issue/TrackerRawInputFactoryTest.java (line 117)

(source file not found at this revision: src/test/java/org/sonar/server/computation/task/projectanalysis/issue/TrackerRawInputFactoryTest.java)

Added src/test/java/org/sonar/server/computation/task/projectanalysis/step/CustomMeasuresCopyStepTest.java (line 106)

(source file not found at this revision: src/test/java/org/sonar/server/computation/task/projectanalysis/step/CustomMeasuresCopyStepTest.java)

Added src/test/java/org/sonar/server/computation/task/projectanalysis/step/CustomMeasuresCopyStepTest.java (line 144)

(source file not found at this revision: src/test/java/org/sonar/server/computation/task/projectanalysis/step/CustomMeasuresCopyStepTest.java)

Added src/test/java/org/sonar/server/issue/IssueDocTesting.java (line 43)

(source file not found at this revision: src/test/java/org/sonar/server/issue/IssueDocTesting.java)

Added src/test/java/org/sonar/server/issue/IssueFieldsSetterTest.java (line 352)

(source file not found at this revision: src/test/java/org/sonar/server/issue/IssueFieldsSetterTest.java)

Added src/test/java/org/sonar/server/issue/IssueFieldsSetterTest.java (line 355)

(source file not found at this revision: src/test/java/org/sonar/server/issue/IssueFieldsSetterTest.java)

Added src/test/java/org/sonar/server/issue/IssueFieldsSetterTest.java (line 361)

(source file not found at this revision: src/test/java/org/sonar/server/issue/IssueFieldsSetterTest.java)

Added src/test/java/org/sonar/server/measure/custom/ws/CustomMeasureValidatorTest.java (line 73)

(source file not found at this revision: src/test/java/org/sonar/server/measure/custom/ws/CustomMeasureValidatorTest.java)

Added src/test/java/org/sonar/server/util/RubyUtilsTest.java (line 91)

(source file not found at this revision: src/test/java/org/sonar/server/util/RubyUtilsTest.java)

Added src/test/java/org/sonar/server/util/RubyUtilsTest.java (line 92)

(source file not found at this revision: src/test/java/org/sonar/server/util/RubyUtilsTest.java)

@romainbrenguier @claude
Tighten S9133 tolerance and add normalize() edge case tests
0c2cf07 
Replace fixed relative tolerance with dynamic precision-based tolerance
derived from the literal's significant digit count. Raise minimum
significant digits from 3 to 4 to avoid false positives on common
domain values like 0.693. Add test cases for underscore-separated
literals, D/d suffixes, leading-dot literals, and zero value.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@romainbrenguier
romainbrenguier force-pushed the new-rule/SONARJAVA-6703-S9133 branch from 4925f8b to 0c2cf07 Compare August 3, 2026 13:49
@romainbrenguier @claude
Improve S9133 test coverage and remove dead code
876d1a1 
Remove unreachable '-' and '+' handling from countSignificantDigits
since Java literal tokens never contain sign characters. Add test
cases for uppercase 'E' scientific notation, '0X' hex prefix, 'F'
float suffix, and leading-dot noncompliant literal.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@gitar-bot

gitar-bot Bot commented Aug 3, 2026

Copy link
Copy Markdown
Code Review ✅ Approved 2 resolved / 2 findings

Implements rule S9133 to detect hard-coded floating-point approximations of mathematical constants, addressing the loose tolerance and normalize() edge case test coverage findings.

✅ 2 resolved
✅ Edge Case: Loose tolerance may flag legitimate 3-digit domain values

📄 java-checks/src/main/java/org/sonar/java/checks/HardcodedMathConstantCheck.java:28-29 📄 java-checks/src/main/java/org/sonar/java/checks/HardcodedMathConstantCheck.java:79-85
With RELATIVE_TOLERANCE=0.002 and only MIN_SIGNIFICANT_DIGITS=3, a literal that matches a constant only in its first ~3 digits (e.g. 2.72 for E, or 0.693 which is a common domain value) is flagged even though the author clearly did not intend the constant's full precision. This can create false positives on genuine domain literals. Consider requiring the literal to match the constant across all of its own significant digits (i.e., the error should be on the order of the literal's own rounding) rather than a fixed relative window, to reduce noise while still catching approximations like 3.14159.

✅ Quality: No tests for normalize() edge cases (underscores, D suffix, leading dot)

📄 java-checks/src/main/java/org/sonar/java/checks/HardcodedMathConstantCheck.java:88-102 📄 java-checks-test-sources/default/src/main/java/checks/HardcodedMathConstantCheckSample.java:1-15
normalize() handles digit separators ("3.14_159"), upper/lower 'd'/'D'/'F' suffixes, and the sample covers 'f' and 'e'/hex, but there is no test exercising an underscore-separated literal, a 'D'/'d' suffixed double, or a leading-dot literal (e.g. ".314159e1"). Add sample lines such as double a = 3.14_159; and double b = 3.14159d; to lock in this normalization behavior against regressions.

Options

Auto-apply is off → Gitar will not commit updates to this branch.
Display: compact → Showing less information.

Comment with these commands to change the behavior for this request:

Auto-apply Compact 
gitar auto-apply:on         

gitar display:verbose

Was this helpful? React with 👍 / 👎 | Gitar

@sonarqube-next

sonarqube-next Bot commented Aug 3, 2026

Copy link
Copy Markdown

Quality Gate passed Quality Gate passed

Issues
0 New issues
0 Fixed issues
0 Accepted issues

Measures
0 Security Hotspots
0 Dependency risks
97.7% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@gitar-bot gitar-bot[bot] gitar-bot[bot] left review comments

At least 1 approving review is required to merge this pull request.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@romainbrenguier